Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggest replacing flake8/isort/black with ruff #543

Merged
merged 7 commits into from
Sep 24, 2024

Conversation

voidlily
Copy link
Contributor

@voidlily voidlily commented Apr 25, 2024

Changes proposed in this pull request:

Ruff is a newer project from only a few years ago that does all the things flake8/isort/black does, but in one package, and is much faster. I think suggesting ruff for new projects would help have less tools to run and set up, and also reduce cycle time by making it run anywhere from slightly to much faster than flake8 depending on the size of the project. This would also pave the way for standardizing on it down the line if adoption is a success.

This PR also adds poetry as another option for a package manager.

Also while I was in here, I found a few blacklist/whitelist references and changed them to allowlist/denylist.

security considerations

The docs portion shouldn't have any security concerns. Additionally, ruff runs locally and you can check changes it makes to your code before committing the changes to git, like flake8 and black formatter.

@voidlily voidlily requested a review from amymok as a code owner April 25, 2024 17:15
@voidlily voidlily changed the title Replace flake8/isort/black with ruff. Suggest replacing flake8/isort/black with ruff Apr 25, 2024
[Ruff](https://github.com/astral-sh/ruff) is a newer project from only a few
years ago that does all the things flake8/isort/black does, but in one package,
and is much faster. I think standardizing on ruff for new projects would help
have less tools to run and set up, and also reduce cycle time by making it run
anywhere from slightly to much faster than flake8 depending on the size of the
project.
low adoption could = dissuasion against, we want to be encouraging
Copy link
Contributor

@alexbielen alexbielen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @voidlily!

I'll approve and merge.

@alexbielen alexbielen merged commit 189475d into 18F:main Sep 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants